fix(miner): reclaim worktree slots by lease age, not cross-container PID liveness by bitfathers94 · Pull Request #7131 · JSONbored/loopover

bitfathers94 · 2026-07-17T23:22:10Z

Summary

worktree-allocator.js's orphan reclaim no longer relies solely on isProcessAlive(owner_pid), which is meaningless across fleet mode's separate containers (each has its own PID namespace over one shared data volume). It now converges on the same age-based convention every sibling shared-lease store already uses (portfolio-queue-expiry.js's DEFAULT_MAX_LEASE_MS/sweepStuckItems, claim-ledger's DEFAULT_MAX_CLAIM_AGE_MS).
Records an owner_host qualifier alongside owner_pid (the gap the issue calls out: the pid was stored "with no host/container qualifier"). The PID-liveness check is retained only as a same-host fast path — it is trusted just when owner_host matches this process's host — so a crashed local owner still frees its slot immediately, while a foreign pid verdict is never trusted.
reclaimOrphanedAllocations now takes an injectable nowMs and a configurable maxLeaseMs (default 6h, well above the 30-minute floor since a worktree lease spans a whole coding attempt), mirroring sweepStuckItems's signature and the existing maxConcurrency option pattern.
A pre-worktree-allocator.js's isProcessAlive PID check is meaningless across fleet-mode's separate containers #7085 store is migrated in place (PRAGMA table_info + ALTER TABLE ADD COLUMN owner_host, the same technique as attempt-log.js's ensureOutcomeColumns); a migrated row keeps owner_host NULL and is governed by the age fallback until re-acquired.

This closes both documented failure modes: a genuinely orphaned slot whose dead pid collides with a live pid in another container is now reclaimed by age; a live worker's slot is no longer stolen the moment another container opens the shared store, because a recent lease is protected regardless of the pid verdict.

Closes #7085

Scope

The PR title follows type(scope): short summary Conventional Commit format.
This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
I linked a currently open issue this PR resolves (Closes #7085).

Validation

git diff --check
npm run typecheck
npm run test:coverage locally — 100% of the changed lines and branches in worktree-allocator.js are covered (both age-threshold branches crossed with both isProcessAlive outcomes, both owner_host presence/match branches, the owner_pid null/dead/alive branches, the unparseable-allocated_at path, the ALTER TABLE migration path, and the invalid-config guards).
npm run build:miner, npm run test:miner-pack, npm run test:miner-deployment-docs-audit, npm run miner:env-reference:check
New behavior has unit tests for every new branch and fallback path, including the two required cross-container regression tests.

If any required check was skipped, explain why:

No UI/MCP/OpenAPI/worker surface is touched (this is a miner local-store fix), so ui:*, test:workers, build:mcp, and ui:openapi:check are not applicable. No env-var reads were added/removed, so no generated reference changes.

Safety

No secrets, wallets, hotkeys, coldkeys, PATs, trust scores, or private evidence exposed.
Public text stays sanitized and low-noise.
No auth/cookie/CORS/GitHub App/Cloudflare/session changes.
No API/OpenAPI/MCP behavior change.
No UI changes.
No changelog edits.

Notes

The new age default (6h) is deliberately more generous than portfolio-queue-expiry.js's 30-minute floor because a single worktree lease covers clone + agent run + push, which can legitimately run for hours; the same-host isProcessAlive fast path still frees a crashed local owner immediately, so the age fallback only ever governs the cross-container case.

…PID liveness Fleet mode runs separate containers over one shared worktree-allocator store, each with its own PID namespace, so reclaimOrphanedAllocations' isProcessAlive(owner_pid) check is meaningless across containers: a dead pid can collide with a live one in another namespace (a genuinely orphaned slot is never reclaimed) and a live pid looks dead the moment another container opens the store (a running attempt's slot is stolen mid-write). Reclaim active slots by allocated_at age past a configurable maxLeaseMs (default 6h), mirroring portfolio-queue-expiry.js's DEFAULT_MAX_LEASE_MS/ sweepStuckItems and claim-ledger's age-based sweep. Record an owner_host qualifier next to owner_pid and only trust the isProcessAlive fast path when the slot was leased on this host, so a crashed local owner still frees its slot immediately while a foreign pid verdict is ignored. reclaimOrphanedAllocations takes an injectable nowMs and maxLeaseMs; a pre-existing store is migrated in place with ALTER TABLE ADD COLUMN. Closes JSONbored#7085

superagent-security · 2026-07-17T23:23:21Z

Superagent didn't find any vulnerabilities or security issues in this PR.

codecov · 2026-07-17T23:30:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.75%. Comparing base (097ac2f) to head (80f5059).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7131   +/-   ##
=======================================
  Coverage   93.75%   93.75%           
=======================================
  Files         692      692           
  Lines       68830    68851   +21     
  Branches    18781    18781           
=======================================
+ Hits        64533    64554   +21     
  Misses       3302     3302           
  Partials      995      995

Flag	Coverage Δ
shard-1	`43.35% <4.00%> (-0.43%)`	⬇️
shard-2	`36.87% <48.00%> (+0.01%)`	⬆️
shard-3	`33.20% <100.00%> (+0.04%)`	⬆️
shard-4	`34.37% <4.00%> (-0.10%)`	⬇️
shard-5	`31.74% <44.00%> (-0.54%)`	⬇️
shard-6	`46.13% <80.00%> (+0.29%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
packages/loopover-miner/lib/worktree-allocator.js	`92.92% <100.00%> (+1.61%)`	⬆️

loopover-orb · 2026-07-17T23:34:34Z

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-17 23:34:50 UTC}

4 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

safe to merge

Review summary
This PR replaces PID-liveness-only orphan reclaim in worktree-allocator.js with an age-based fallback (default 6h) plus a same-host-only PID fast path, adding an `owner_host` column with an in-place migration for pre-existing stores. The logic in `isSlotOrphaned` correctly prioritizes age (reclaims regardless of host) and only trusts `isProcessAlive` when `owner_host` matches the current host, which matches the stated intent of fixing cross-container PID collisions. The migration path (`ensureOwnerHostColumn`) mirrors the existing `ensureOutcomeColumns` convention and is well covered by a dedicated legacy-store test.

Nits — 5 non-blocking

worktree-allocator.js: `normalizeMaxLeaseMs` and `normalizeHostId` duplicate validation patterns already used elsewhere in the file (e.g. `normalizeMaxConcurrency`) — fine as-is, just noting the repeated shape rather than a shared helper.
The 6h `DEFAULT_MAX_LEASE_MS` is a meaningful behavior change (previously a dead process's slot was freed almost immediately via PID check; now a foreign-host dead owner's slot can be held up to 6h) — worth confirming this tradeoff is acceptable for fleet-mode throughput, though the same-host fast path mitigates the common case.
test/unit/miner-worktree-allocator-lease-expiry.test.ts is thorough but long (226 lines) for one behavior change; consider whether some of the near-duplicate cross-container cases could be table-driven.
Consider logging/telemetry when a slot is reclaimed purely by age vs. by the same-host pid fast path, so operators can distinguish a genuinely stuck cross-container lease from normal churn.
Document the 6h default and its override (`maxLeaseMs`) in DEPLOYMENT.md alongside the fleet-mode container description referenced in the code comments.

Decision drivers

✅ Code review — No blockers (1 reviewer)
✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#7085
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 0 registered-repo PR(s), 0 merged, 0 issue(s).
Contributor context	ℹ️ No public Gittensor match	bitfathers94; not a blocker.
Improvement	✅ Minor	risk: clean · value: minor · LLM: significant

Review context

Author: bitfathers94
Role context: outside_contributor
Public audience mode: oss maintainer
Lane context: Repository is configured for direct PR review.
Public profile languages: not available
Contributor context: Public profile only; not a blocker.
PR-specific overlap: none found.

Contributor next steps

Keep the PR focused and include validation evidence before maintainer review.

Signal definitions

Related work = same linked issue, overlapping active PRs, or title/path similarity.
Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

@loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
@loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

Re-run LoopOver review

loopover-orb

LoopOver approves — the gate is satisfied and CI is green.

bitfathers94 requested a review from JSONbored as a code owner July 17, 2026 23:22

github-actions Bot deployed to preview/pr-7131 July 17, 2026 23:25 View deployment

loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 17, 2026

loopover-orb Bot assigned bitfathers94 Jul 17, 2026

loopover-orb Bot approved these changes Jul 17, 2026

View reviewed changes

loopover-orb Bot merged commit 237530a into JSONbored:main Jul 17, 2026
16 checks passed

This was referenced Jul 17, 2026

chore(main): release engine-and-dependents libraries #7133

Merged

chore(main): release engine-and-dependents libraries #7144

Merged

chore(main): release engine-and-dependents libraries #7211

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(miner): reclaim worktree slots by lease age, not cross-container PID liveness#7131

fix(miner): reclaim worktree slots by lease age, not cross-container PID liveness#7131
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
bitfathers94:fix/issue-7085

bitfathers94 commented Jul 17, 2026

Uh oh!

superagent-security Bot commented Jul 17, 2026

Uh oh!

codecov Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

loopover-orb Bot commented Jul 17, 2026 •

edited

Loading

✅ LoopOver review result - approve/merge recommended

Uh oh!

loopover-orb Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

bitfathers94 commented Jul 17, 2026

Summary

Scope

Validation

Safety

Notes

Uh oh!

superagent-security Bot commented Jul 17, 2026

Uh oh!

codecov Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

loopover-orb Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ LoopOver review result - approve/merge recommended

Uh oh!

loopover-orb Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Jul 17, 2026 •

edited

Loading

loopover-orb Bot commented Jul 17, 2026 •

edited

Loading